Skip to content

Manifest v3 update#528

Draft
natpicone wants to merge 2 commits intomasterfrom
manifest-upd
Draft

Manifest v3 update#528
natpicone wants to merge 2 commits intomasterfrom
manifest-upd

Conversation

@natpicone
Copy link
Collaborator

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the web extension build and background runtime to Manifest V3 by transforming the generated manifest per-platform and replacing the MV2 AngularJS background page bootstrap with an MV3-compatible background entry that uses a manual DI container and lightweight AngularJS service shims.

Changes:

  • Transform generated manifest.json to MV3 for Chromium and Firefox, and stop copying background.html.
  • Replace AngularJS background module bootstrap with MV3 background entry points + a manual DI container.
  • Add AngularJS service shims and a background-friendly alert notification hook.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
webpack/firefox.config.js Adds MV3 manifest transform + Firefox-specific settings; removes background.html copy.
webpack/chromium.config.js Adds MV3 manifest transform for Chromium; removes background.html copy.
src/modules/webext/webext-background/webext-background.service.ts Removes AngularJS usage for install handling; switches bookmark service abstraction; removes angular.isUndefined usage.
src/modules/webext/webext-background/background-container.ts New manual DI container for MV3 background context.
src/modules/webext/webext-background/angular-shims.ts New $q/$http/$timeout/$interval/$injector + angular global shim for service-worker execution.
src/modules/webext/shared/webext-platform/webext-platform.service.ts Removes AngularJS dependency; supports browser.action in MV3; background-context messaging shortcut.
src/modules/webext/firefox/firefox-background/firefox-background.module.ts New MV3 Firefox background entry point using the manual DI container.
src/modules/webext/chromium/chromium-background/chromium-background.module.ts New MV3 Chromium service-worker entry point using the manual DI container.
src/modules/shared/alert/alert.service.ts Adds onAlertChanged callback to support background notifications on alerts.
Comments suppressed due to low confidence (2)

src/modules/webext/chromium/chromium-background/chromium-background.module.ts:40

  • MV3 service workers can be started for events like runtime.onMessage without firing onStartup; with the current gating, init() may never run in those wake-ups. Consider initializing (or ensuring a one-time init) at service-worker startup so required state (e.g., icon/title refresh, update checks, enabling sync) is consistently set whenever the worker is launched.
// Register event handlers synchronously (required for MV3 service workers)
let startupInitiated = false;

browser.runtime.onInstalled.addListener((details) => {
  if (startupInitiated) return;
  startupInitiated = true;
  backgroundSvc.onInstall(details.reason);
});

browser.runtime.onStartup.addListener(() => {
  if (startupInitiated) return;
  startupInitiated = true;
  backgroundSvc.init();
});

src/modules/webext/firefox/firefox-background/firefox-background.module.ts:40

  • MV3 background scripts/service workers may be woken by events other than onStartup/onInstalled (e.g., runtime.onMessage, alarms). With the current startupInitiated gating, init() may not run on those wake-ups. Consider triggering a one-time init at script startup so the background is always initialized whenever the worker starts.
// Register event handlers synchronously (required for MV3 background scripts)
let startupInitiated = false;

browser.runtime.onInstalled.addListener((details) => {
  if (startupInitiated) return;
  startupInitiated = true;
  backgroundSvc.onInstall(details.reason);
});

browser.runtime.onStartup.addListener(() => {
  if (startupInitiated) return;
  startupInitiated = true;
  backgroundSvc.init();
});

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +34
// Move host patterns from optional_permissions to optional_host_permissions
manifest.optional_host_permissions = manifest.optional_permissions;
delete manifest.optional_permissions;
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says to move only host patterns from optional_permissions to optional_host_permissions, but the implementation moves the entire optional_permissions array and deletes optional_permissions. In MV3, optional_permissions is still used for non-host permissions; consider splitting the array (host patterns vs regular permissions) and only moving the host patterns, leaving any non-host optional permissions intact.

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +35
// Move host patterns from optional_permissions to optional_host_permissions
manifest.optional_host_permissions = manifest.optional_permissions;
delete manifest.optional_permissions;

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as Chromium config: this moves the entire optional_permissions array into optional_host_permissions and deletes optional_permissions, despite the comment indicating only host patterns should move. To avoid breaking future non-host optional permissions, split and move only host patterns while preserving optional_permissions for non-host entries.

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +15
// Set up angular shim before any service code runs
setupAngularShim();

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setupAngularShim() is called in the module body, but all imports (including services that may reference angular or import the real AngularJS package) are evaluated before this runs. If the intent is to rely on the shim in the MV3 background context, move shim setup to a side-effect import that executes before other modules, or ensure background dependencies do not import angular at all.

This issue also appears on line 27 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines +13 to 15
// Set up angular shim before any service code runs
setupAngularShim();

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setupAngularShim() runs after module imports have already been evaluated. If any imported service relies on a global angular shim (or if importing AngularJS would fail in a service worker), this is too late. Consider making the shim a side-effect import executed before any other background dependencies, or remove/alias all import angular from 'angular' usage for the MV3 background bundle.

This issue also appears on line 27 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines +230 to +236
get: (name: string): any => {
const svc = services.get(name);
if (!svc) {
throw new Error(`Service '${name}' not registered in background injector`);
}
return svc;
},
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$injectorFactory.get() treats any falsy registered value (e.g., 0/false/"") as “not registered” because it checks if (!svc). This should check services.has(name) instead, otherwise valid registrations can be incorrectly rejected.

Copilot uses AI. Check for mistakes.
Comment on lines +70 to +76
$timeout.cancel = (promise: any): boolean => {
if (promise && promise.$$timeoutId != null) {
clearTimeout(promise.$$timeoutId);
return true;
}
return false;
};
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$timeout.cancel() clears the timer but leaves the returned promise permanently pending. AngularJS $timeout typically rejects/cancels the promise so callers can unblock; consider rejecting/settling the promise on cancel (or returning a cancellable promise implementation) to avoid dangling awaits and resource leaks.

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +129
// 11. ApiXbrowsersyncService (depends on $injector, $http, $q, NetworkService, StoreService, UtilityService)
const apiSvc = new ApiXbrowsersyncService(injector, $http, $q, networkSvc, storeSvc, utilitySvc);
injector.register('ApiXbrowsersyncService', apiSvc);
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiXbrowsersyncService is instantiated in the MV3 background container, but it imports DOMPurify/marked and uses DOMParser for formatting service messages. Those DOM-dependent dependencies can be problematic in a service worker context. Consider moving the markdown/DOM sanitization logic into a UI-only helper (or lazy-loading it inside formatServiceInfo) so the background can safely use the API client without DOM requirements.

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +58
export const createBackgroundContainer = (config: BackgroundPlatformConfig): BackgroundContainer => {
// Create AngularJS shims
const $q = $qFactory();
const $timeout = $timeoutFactory();
const $interval = $intervalFactory();
const $http = $httpFactory();
const $log = $logFactory();
const $rootScope = $rootScopeFactory();
const $location = $locationFactory();
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This container aims to run background code without AngularJS, but many of the services it instantiates (and the platform BookmarkService implementations it receives) still import angular from 'angular', which will pull AngularJS into the MV3 background bundle and bypass the shim approach. Consider aliasing angular to a lightweight shim for the background build, or refactoring those services to avoid importing AngularJS (and ensuring shim setup happens before any usage).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants